Refactor permission check redirect#2109
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors permission handling in several React UI routes by introducing a shared hook that checks access and redirects when a required permission is missing.
Changes:
- Added
useCheckAccessAndRedirect(permission)hook to centralize permission-check + redirect logic. - Applied the new hook to OpenOps Tables, OpenOps Analytics, and Flows routes to prevent rendering when access is missing.
- Updated side-menu link metadata to lock Tables/Analytics links based on write permissions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-ui/src/app/common/hooks/authorization-hooks.ts | Adds useCheckAccessAndRedirect hook for permission-based redirects. |
| packages/react-ui/src/app/routes/openops-tables/index.tsx | Uses the new access+redirect hook for Tables route. |
| packages/react-ui/src/app/routes/openops-analytics/index.tsx | Uses the new access+redirect hook for Analytics route. |
| packages/react-ui/src/app/routes/flows/index.tsx | Adds access+redirect gate to Flows list route. |
| packages/react-ui/src/app/routes/flows/id/index.tsx | Adds access+redirect gate to Flow builder route. |
| packages/react-ui/src/app/features/templates/components/select-flow-template-dialog-content.tsx | Replaces local permission check with redirecting hook in a dialog context. |
| packages/react-ui/src/app/features/navigation/lib/menu-links-hook.ts | Locks Tables/Analytics menu links based on permissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const hasWriteFlowPermission = useCheckAccessAndRedirect( | ||
| Permission.WRITE_FLOW, | ||
| ); |
There was a problem hiding this comment.
This dialog content previously used checkAccess only to disable the “use template” action; switching to useCheckAccessAndRedirect will navigate the user away from the current page when they lack WRITE_FLOW, even though the UI already supports a read-only experience (it just omits useTemplate). Consider using useAuthorization().checkAccess here (no redirect), or make the redirect hook optional/configurable so non-route components don’t cause unexpected navigation.
| @@ -51,6 +53,10 @@ const FlowBuilderPage = () => { | |||
| refetchOnWindowFocus: 'always', | |||
There was a problem hiding this comment.
useQuery will still fetch the flow even when hasAccess is false (the redirect happens in an effect after render). To avoid unnecessary API calls (and caching potentially sensitive data client-side), gate the query with an enabled condition that includes hasAccess (and flowId).
| refetchOnWindowFocus: 'always', | |
| refetchOnWindowFocus: 'always', | |
| enabled: Boolean(hasAccess && flowId), |
|
| if (!hasAccess) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Do we really need it? In theory it is not reachable as user will be redirected. But we can keep it to be more confident
Fixes OPS-3916